Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add "HumanBinaryData" as an alternative to "Base64UrlSafeData" #354

Merged
merged 8 commits into from
Oct 2, 2023

Conversation

micolous
Copy link
Collaborator

@micolous micolous commented Sep 29, 2023

Related to issue #352, it doesn't fix things, but it's a start.

HumanBinaryData uses a more efficient "bytes" format when using non-human-readable serialisation formats (like CBOR). The deserialiser still uses deserialize_any – I haven't got a great way around this yet.

I've done a few things differently to Base64UrlSafeData; for example, this doesn't implement Display or FromStr – they're different to what Vec<u8> does, and we should move away from this (this will be the focus of #356).

It also implements Deref and DerefMut, which should make the type more transparent – and all of the common impls are in macros.

I've added bytes support to Base64UrlSafeData, so it can read what HumanBinaryData produces. Unfortunately this makes migration a two-step process.

While we're here, this makes serde_json a dev-dependency of base64urlsafedata, because it's only used in tests; and this also adds a bunch more tests for expected input formats.

  • cargo test has been run and passes
  • documentation has been updated with relevant examples (if relevant)

@micolous
Copy link
Collaborator Author

micolous commented Sep 29, 2023

Clippy appears to fail due to checks in 1.70.0, possibly triggered by #348. #355 should fix that.

Copy link
Member

@Firstyear Firstyear left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a great idea and a great way to approach this. Great work @micolous <3

@micolous micolous merged commit c20e83f into kanidm:master Oct 2, 2023
33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants